Skip to content

Use CMake EXPORT #8954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 17 commits into
base: gh/swolchok/522/head
Choose a base branch
from
Draft

Use CMake EXPORT #8954

wants to merge 17 commits into from

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Mar 5, 2025

The CMake EXPORT feature makes target_compile_options/target_link_libraries etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. We are currently working around our lack of EXPORT by (incompletely and therefore incorrectly!) repeating our configuration in tools/cmake/executorch-config.cmake.

I was previously stalled here by XNNPACK's lack of use of EXPORT, but I found that this can be worked around by building those targets with ExternalProject!

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Mar 5, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/8954

Note: Links to docs will display an error until the docs builds have been completed.

❌ 25 New Failures, 7 Pending, 1 Unrelated Failure

As of commit 2e1d1e4 with merge base 4d7f9ca (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

swolchok added a commit that referenced this pull request Mar 5, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: e3e5b36
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 5, 2025
@swolchok swolchok marked this pull request as draft March 5, 2025 01:40
Copy link

github-actions bot commented Mar 5, 2025

This PR needs a release notes: label

If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@swolchok
Copy link
Contributor Author

have an update cooking, need to make sure check whether warnings I'm seeing about macOS version mismatch during install_executorch.sh are new and if so problematic

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 14, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: af3cc8d
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
swolchok added 2 commits July 16, 2025 17:50
[ghstack-poisoned]
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 17, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: 16c0343
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
@swolchok swolchok changed the base branch from gh/swolchok/515/head to gh/swolchok/516/head July 17, 2025 00:52
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 17, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: 2f0620d
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 17, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: 742fd2f
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 17, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: d71d97f
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
@swolchok swolchok changed the base branch from gh/swolchok/516/head to gh/swolchok/517/head July 17, 2025 22:30
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 18, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: f733c8c
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 18, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: 2712a70
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
@swolchok
Copy link
Contributor Author

test-binary-size-linux regression is real. text size was 35810 before and is now 43358.

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 19, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: 7d8ea62
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 19, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: fb57c08
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 19, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: cdcf948
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
@swolchok
Copy link
Contributor Author

I think the size regression is the same old "now we are counting register_prim_ops" fake regression from #12320, and I think we should just accept it and move the goalpost, since it is caused by building the size test the same way we build internally. If this really isn't what we want, then we should stop using force_load/whole_archive on the top-level ExecuTorch target, but I strongly suspect that that is not what we want. CC @mergennachin

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 21, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: 07c4720
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
@swolchok swolchok changed the base branch from gh/swolchok/517/head to gh/swolchok/522/head July 21, 2025 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants